Document how to connect a remote BMC in the Tilt dev environment#760
Document how to connect a remote BMC in the Tilt dev environment#760
Conversation
github-actions
Bot
added
size/M
documentation
xkonni
force-pushed
the
doc/dev_remote_bmc
branch
from
8d92bde to
6d11946
Compare
📝 WalkthroughWalkthroughAdded documentation and formatting updates to Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/development/dev_setup.md`:
- Around line 116-129: The title and explanation are backwards: the Tiltfile
manager arg `--insecure=false` actually enables TLS/HTTPS (insecure=true
disables TLS and uses HTTP). Update the section title and explanatory sentence
to state that `--insecure=false` enables TLS/HTTPS (or conversely that
`--insecure=true` disables TLS), and clarify that when connecting to a real BMC
on port 443 you should use `--insecure=false`; keep the `new_args -> metal`
entry with the `--insecure` flag as shown.
- Around line 83-100: The example BMC resource should explicitly include the
optional scheme field to indicate HTTPS when using port 443: update the BMC
example (kind: BMC) to add scheme: https alongside protocol.name: Redfish and
port: 443 so the InlineEndpoint/scheme is clear; reference the scheme field in
the same block where port: 443 is declared to make the intent explicit (e.g.,
under the protocol/endpoint section that currently contains protocol.name and
port).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 16f30b11-66d0-45a3-9277-927f1f96dd5d
📒 Files selected for processing (1)
docs/development/dev_setup.md
xkonni
force-pushed
the
doc/dev_remote_bmc
branch
from
6d11946 to
42fdbad
Compare
xkonni
force-pushed
the
doc/dev_remote_bmc
branch
from
39f7e05 to
f428390
Compare
xkonni
force-pushed
the
doc/dev_remote_bmc
branch
from
f428390 to
95bb6c8
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/development/dev_setup.md (1)
145-157: Consolidate duplicatedBMCSecretmanifest example.The same manifest is documented twice. Keep one canonical block and reference it later to reduce drift and keep this guide tighter.
Also applies to: 182-194
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/development/dev_setup.md` around lines 145 - 157, There are two identical examples of the BMCSecret manifest (the YAML showing apiVersion: metal.ironcore.dev/v1alpha1, kind: BMCSecret, metadata: name, data: username/password) duplicated in the doc; remove the second duplicate and keep a single canonical BMCSecret YAML block (the one with apiVersion/ kind/ metadata/ data fields) and replace the removed copy with a short cross-reference comment or link to the canonical section so later steps refer to that single manifest example.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/development/dev_setup.md`:
- Around line 145-157: There are two identical examples of the BMCSecret
manifest (the YAML showing apiVersion: metal.ironcore.dev/v1alpha1, kind:
BMCSecret, metadata: name, data: username/password) duplicated in the doc;
remove the second duplicate and keep a single canonical BMCSecret YAML block
(the one with apiVersion/ kind/ metadata/ data fields) and replace the removed
copy with a short cross-reference comment or link to the canonical section so
later steps refer to that single manifest example.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f51e4165-bb62-43f5-9f3f-ec5f337e5976
📒 Files selected for processing (1)
docs/development/dev_setup.md
xkonni
force-pushed
the
doc/dev_remote_bmc
branch
from
95bb6c8 to
5d1c455
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/development/dev_setup.md`:
- Around line 145-157: Consolidate the duplicate BMCSecret examples by keeping
the YAML example and base64-note in Step 2 and replacing the second repeated
YAML in Step 4 with instructions to apply the file; specifically, in the Step 2
block that shows the BMCSecret (apiVersion: metal.ironcore.dev/v1alpha1, kind:
BMCSecret, metadata: name: <node-name>, data: username/password), add the
explicit note that username and password must be base64-encoded (and show the
echo -n '<value>' | base64 instruction), tell users to save it as
bmcsecret-<node-name>.yaml but not to apply it yet, then in the later step where
the duplicate YAML appears remove that YAML and instead instruct users to run
kubectl apply -f bmcsecret-<node-name>.yaml against the Kind cluster once the
manager is running.
- Around line 159-172: Update the docs block that describes enabling HTTPS to
note that the --insecure flag is deprecated and instruct users to use the new
flags instead; replace the guidance showing --insecure=false with a
recommendation to set --protocol=https and --skip-cert-validation=false in the
Tiltfile settings (referencing the same settings/new_args/metal example
structure), and update the example snippet and description to show the preferred
configuration and mention the deprecation of --insecure so readers won’t follow
the legacy flag.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f01a493c-6a32-4163-9631-a554965803d0
📒 Files selected for processing (1)
docs/development/dev_setup.md
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/development/dev_setup.md (1)
163-179: Clarify certificate validation behavior for development environments.The guidance to use
--skip-cert-validation=falseis correct for production-grade BMCs with trusted certificates, but most development and lab BMCs use self-signed certificates. Users following this example will encounter TLS handshake errors without understanding why.Consider adding a note explaining when to use each setting:
📝 Suggested clarification
#### 3. Enable HTTPS for the BMC connection The `--insecure` flag is deprecated. Use `--protocol` and `--skip-cert-validation` instead. -For a real BMC that uses HTTPS on port 443, configure the manager to use secure HTTPS with certificate validation enabled in the `Tiltfile`: +For a real BMC that uses HTTPS on port 443, configure the manager in the `Tiltfile`: ```python settings = { "new_args": { "metal": [ # ... "--protocol=https", - "--skip-cert-validation=false", + "--skip-cert-validation=true", # or false if BMC has a trusted certificate ], } }
+> Note: Set
--skip-cert-validation=truewhen the BMC uses a self-signed certificate (common in development environments). Usefalseonly if the BMC certificate is signed by a CA trusted by the manager pod.</details> Alternatively, if the debug manager image with `ca-certificates` is intended to address this (by allowing users to install custom CAs), that workflow should be referenced here. <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/development/dev_setup.mdaround lines 163 - 179, Update the Tiltfile
example under the HTTPS BMC section to clarify certificate validation behavior:
in the settings/new_args["metal"] example mention that --skip-cert-validation
can be true for development/lab BMCs using self-signed certs and false only for
BMCs with CA-trusted certs, and add a short note that explains when to use each
value (or reference the debug manager CA workflow if applicable) so users know
to set "--skip-cert-validation=true" for self-signed certs and
"--skip-cert-validation=false" for trusted certificates.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In@docs/development/dev_setup.md:
- Around line 163-179: Update the Tiltfile example under the HTTPS BMC section
to clarify certificate validation behavior: in the settings/new_args["metal"]
example mention that --skip-cert-validation can be true for development/lab BMCs
using self-signed certs and false only for BMCs with CA-trusted certs, and add a
short note that explains when to use each value (or reference the debug manager
CA workflow if applicable) so users know to set "--skip-cert-validation=true"
for self-signed certs and "--skip-cert-validation=false" for trusted
certificates.</details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Organization UI **Review profile**: CHILL **Plan**: Pro **Run ID**: `d1fa83fb-c094-4c59-8a0d-86053d269d8d` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 95bb6c874949be79be34ccbf596fd9aa96f3c8e7 and 49a5864edf9fcf31259c42ffcddb57ae16398db6. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `docs/development/dev_setup.md` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
5 participants
Proposed Changes
Fixes
Summary by CodeRabbit